Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format call chains! #1356

Merged
merged 2 commits into from
Jan 11, 2024
Merged

Format call chains! #1356

merged 2 commits into from
Jan 11, 2024

Conversation

munificent
Copy link
Member

This handles unsplit call chains, fully split ones, as well as block-style calls, like:

target.call(
  argument,
);

This doesn't fully implement the proposal I sketched out because it only allows a single block call. There's a long TODO(tall) comment about whether we want to do that, but I figured it makes sense to do that as a separate iteration since there is so much in here.

But it implements most of it, and I've migrated all of the existing tests over. It ended up being both simpler than the old formatter while also producing better output.

The old formatter has a restriction where the indentation of a piece of code must be fixed regardless of which way it splits. It can't handle:

// Block style, so indent argument +2:
target.property.call(
  argument
);

// Can't do block style, so indent +6:
target
    .slightlyLongerProperty
    .call(
      argument
    );

That leads to it sometimes producing gross output like:

target
    .longProperty
    .anotherLongProperty
    .method(
  wtfIsGoingOnHere,
);

To try to avoid that, it has lots of heuristics during Chunk construction to try to not end up with output like that during line splitting.

The new formatter doesn't need any of those heuristics because it can freely choose indentation during line splitting. 🎉

This handles unsplit call chains, fully split ones, as well as
block-style calls, like:

```
target.call(
  argument,
);
```

This doesn't fully implement the proposal I sketched out because it only
allows a single block call. There's a long TODO(tall) comment about
whether we want to do that, but I figured it makes sense to do that as a
separate iteration since there is so much in here.

But it implements most of it, and I've migrated all of the existing
tests over. It ended up being both simpler than the old formatter while
also producing better output.

The old formatter has a restriction where the indentation of a piece of
code must be fixed regardless of which way it splits. It can't handle:

```
// Block style, so indent argument +2:
target.property.call(
  argument
);

// Can't do block style, so indent +6:
target
    .slightlyLongerProperty
    .call(
      argument
    );
```

That leads to it sometimes producing gross output like:

```
target
    .longProperty
    .anotherLongProperty
    .method(
  wtfIsGoingOnHere,
);
```

To try to avoid that, it has lots of heuristics during Chunk
construction to try to not end up with output like that during line
splitting.

The new formatter doesn't need any of those heuristics because it can
freely choose indentation during line splitting.
@munificent munificent merged commit f9181eb into main Jan 11, 2024
7 checks passed
@munificent munificent deleted the format-call-chains branch January 11, 2024 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants